-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[UX] Improve page-load axis #78392
[UX] Improve page-load axis #78392
Conversation
Pinging @elastic/apm-ui (Team:apm) |
Pinging @elastic/uptime (Team:uptime) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a few questions/corrections, but looks good!
numeral(pageViewsTotal).format('0,0') | ||
) : ( | ||
<EuiToolTip content={numeral(pageViewsTotal).format('0,0')}> | ||
<>{numeral(pageViewsTotal).format('0 a')}</> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why React.Fragment
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EuiTitle goes crazy if simple text is provided as child, it needs element.
x-pack/plugins/apm/server/lib/rum_client/get_page_load_distribution.ts
Outdated
Show resolved
Hide resolved
@@ -15,24 +15,38 @@ import { | |||
|
|||
export const MICRO_TO_SEC = 1000000; | |||
|
|||
const NUMBER_OF_PLD_STEPS = 100; | |||
|
|||
export function microToSec(val: number) { | |||
return Math.round((val / MICRO_TO_SEC + Number.EPSILON) * 100) / 100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just noticing this, but why the usage of Number.EPSILON
?
Given that we are using Math.round
on the result, is adding Number.EPSILON
to val/MICRO_TO_SEC
going to have any significant outcome on the output of Math.round
? Sorry if I'm missing something, it just seemed strange to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think i wanted to two decimal places here without rounding out the values.
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
Co-authored-by: Elastic Machine <[email protected]>
* master: (36 commits) [I18n] fix eui tokens (elastic#78951) Changed the color of the confirm button in trusted app deletion dialog. (elastic#78768) Make the actual Vislib import async (elastic#78949) Fix ML conditionals links Cypress tests (elastic#78568) [Drilldowns][Docs] Communicate the visualization types that support drilldowns (elastic#78761) [UX] Improve page-load axis (elastic#78392) [SECURITY SOLUTIONS] Map embeddable working with index patterns selection (elastic#78610) Data plugin README (elastic#78750) [TSVB] Request validation error: [panels.0.series.0.metrics.0.percentiles.1.value] (elastic#79009) fixing api test (elastic#78964) [Task names in TaskManager] Rename "telemetry" to "usage" (elastic#78129) [Loggers] Rename "telemetry" to "usage" (elastic#78130) [Usage Collection] [schema] `ui_metric` (elastic#78827) [Actions][Jira] Set parent issue for Sub-task issue type (elastic#78772) [Discover] Unskip doc link functional test (elastic#78600) [ML] Functional tests - stabilize calendar edit tests (elastic#78950) [UX] Improve page responsive (elastic#78759) [QA][Code Coverage] Team Assignment Docs Update (elastic#78890) [ML] Migrate machine learning URLs to BrowserRouter format for APM, Security, and Infra (elastic#78209) [ts] enable "resolveJsonModule" and disable existing failures (elastic#78855) ...
Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
Summary